Conversation
|
|
||
| #define STACK_SIZE (16*6 + 8*8 + 6*8 + (STACK_LOCS) * 8) | ||
| #define MLK_STACK_SIZE (16*6 + 8*8 + 6*8 + (STACK_LOCS) * 8) | ||
| #define STACK_BASE_GPRS (6*8) |
There was a problem hiding this comment.
We shouldn't namespace STACK_SIZE, but not the other macros; either all of them, or none.
hanno-becker
left a comment
There was a problem hiding this comment.
If we do this we should namespace all macros in the assembly files.
Note that the lack of namespacing here is of little effect since all affected files are in dev/ and not in the main source tree.
a7001df to
59a18fd
Compare
e96e223 to
0878495
Compare
f855045 to
b6a4787
Compare
4db09cb to
5f64f67
Compare
0b7e708 to
17d96d2
Compare
CBMC Results (ML-KEM-512)Full Results (191 proofs)
|
CBMC Results (ML-KEM-1024)Full Results (191 proofs)
|
CBMC Results (ML-KEM-768)Full Results (191 proofs)
|
17d96d2 to
4be4259
Compare
|
@willieyz Could you rebase this please? |
Hello @hanno-becker, |
4be4259 to
68eabf4
Compare
|
Hello, @hanno-becker , |
mkannwischer
left a comment
There was a problem hiding this comment.
Thanks @willieyz. Generally I support this change for consistency and I checked that you have not missed any non-namespaced macros.
A couple of comments got wrongly changed in the x86_64 code (see below), please change them back.
Since the scope of this PR has changed, please update the commit message and PR description to reflect that.
| // | ||
| // Notes: | ||
| // - We exit early if we find the required number of good values, | ||
| // - We exit early if we find the required number of MLK_GOOD values, |
There was a problem hiding this comment.
| // - We exit early if we find the required number of MLK_GOOD values, | |
| // - We exit early if we find the required number of good values, |
| // occupies a corresponding 16-bit element of `MLK_VALS` xmm register, | ||
| // 2. Compute an 8-bit value `MLK_GOOD` such that | ||
| // MLK_GOOD[i] = MLK_VALS[i] < MLKEM_Q ? 1 : 0, for i in [0, 7], | ||
| // 3. Shuffle the elements in `MLK_VALS` such that all MLK_GOOD elements |
There was a problem hiding this comment.
| // 3. Shuffle the elements in `MLK_VALS` such that all MLK_GOOD elements | |
| // 3. Shuffle the elements in `MLK_VALS` such that all good elements |
| movq $0, cnt // cnt counts the number of good values we've found. | ||
| movq $0, pos // pos is the current position in the input buffer. | ||
| movq $0x5555, pext_mask // 0x5555 mask to extract every second bit. | ||
| movq $0, MLK_CNT // MLK_CNT counts the number of MLK_GOOD values we've found. |
There was a problem hiding this comment.
| movq $0, MLK_CNT // MLK_CNT counts the number of MLK_GOOD values we've found. | |
| movq $0, MLK_CNT // MLK_CNT counts the number of good values we've found. |
| pinsrq $1, %rax, MLK_BOUND | ||
|
|
||
| // Broadcast 12-bit mask 0xFFF to all 16-bit elements of bound reg. | ||
| // Broadcast 12-bit mask 0xFFF to all 16-bit elements of MLK_BOUND reg. |
There was a problem hiding this comment.
| // Broadcast 12-bit mask 0xFFF to all 16-bit elements of MLK_BOUND reg. | |
| // Broadcast 12-bit mask 0xFFF to all 16-bit elements of MLK_AND_MASK reg. |
|
Marking as draft for now. Please mark it as ready when my comments have been addressed. |
|
@willieyz - gentle ping. Could you please get this updated so we can get it merged? |
68eabf4 to
bddb1d2
Compare
Hello @mkannwischer, |
bddb1d2 to
bee554b
Compare
bee554b to
0110138
Compare
8558864 to
d5bf6d3
Compare
Hello, @mkannwischer , Apologies for the extra review effort, and thank you for your patience, could you please kindly take another look when you have time? |
While you are at it, can you change to the most recent SLOTHY version (0.2.0)? You should be able to just use the version not the commit. |
0920843 to
c68e02e
Compare
- Assembly files may define local macros (e.g. `STACK_SIZE`, `KECCAK_F1600_ROUNDS`) without the `MLK_` prefix, risking name clashes with symbols in consuming libraries. - To be consistent with the rest of the codebase, all such macros are prefixed with `MLK_`, bringing them in line with the established namespacing convention. Signed-off-by: willieyz <willie.zhao@chelpis.com>
c68e02e to
88208ec
Compare
|
After discussing with @hanno-becker, we have concluded that the loss in readability in |
Resolves: Namespace
STACK_SIZEin various assembly files #1395Assembly files may define local macros (e.g. STACK_SIZE, KECCAK_F1600_ROUNDS) without the MLK_ prefix, risking name clashes with symbols in consuming libraries.
To be consistent with the rest of the codebase, all such macros are prefixed with MLK_, bringing them in line with the established namespacing convention.